fix(sdk): restore cache token fallbacks in _cache_buckets for Kimi-K2 and Anthropic streaming - #5019
Draft
all-hands-bot wants to merge 1 commit into
Draft
all-hands-bot wants to merge 1 commit into
all-hands-bot wants to merge 1 commit into
Conversation
PR #4968 rewrote _cache_buckets to only check prompt_tokens_details, dropping fallback paths for Kimi-K2 (cached_tokens in model_extra) and Anthropic (_cache_read_input_tokens / _cache_creation_input_tokens in private attrs). This silently zeroed cache-read and cache-write token accounting for those providers, producing incorrect cost metrics and telemetry spans. Restore the fallback chain while keeping the cleaner type-safe access: 1. prompt_tokens_details.cached_tokens / cache_creation_tokens 2. _cache_read_input_tokens / _cache_creation_input_tokens (private) 3. cached_tokens (model_extra, Kimi-K2 / DeepSeek) Added regression tests for each provider shape. Co-authored-by: openhands <openhands@all-hands.dev>
Contributor
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HUMAN:
AGENT:
Why
Source PR reviewed: #4968 (
fix(sdk): generate titles with Responses and subscription streaming), authored by @neubig, approved by @enyst.Applicable risk lenses: #1 (Boundary contracts and alternate valid inputs), #10 (Negative-space and silent-success cases), #8 (Credentials, redaction, and sensitive persistence boundaries — cost telemetry accuracy).
Deterministic reproduction and impact:
PR #4968 rewrote
Telemetry._cache_buckets()to only checkprompt_tokens_details.cached_tokens/cache_creation_tokens. It dropped all fallback paths that previously handled provider-specific usage shapes:usage.cached_tokensdirectly on theUsageobject (viamodel_extra), not insideprompt_tokens_details. After fix(sdk): generate titles with Responses and subscription streaming #4968,_cache_bucketsreturns(0, 0)becauseprompt_tokens_detailsisNone._cache_read_input_tokensand_cache_creation_input_tokensduring streaming, whereprompt_tokens_detailsmay be absent. After fix(sdk): generate titles with Responses and subscription streaming #4968, these are silently ignored.Impact: Cache-read and cache-write token accounting is silently zeroed for Kimi-K2, DeepSeek, and Anthropic streaming responses. This produces incorrect cost metrics (cache-read tokens are typically discounted 10x) and misleading telemetry spans. The bug is silent — no exception is raised, the feature just returns wrong numbers.
Root cause: The refactored
_cache_bucketsreplaced the multi-sourcegetattrfallback chain with a single-path check onprompt_tokens_details, without accounting for providers that populate cache token counts through alternate attributes.Proof that the regression test fails on unmodified main:
Why the original tests did not catch the case: PR #4968's test suite only tested cache token extraction via
prompt_tokens_details(the standard OpenAI shape). No test exercised the Kimi-K2cached_tokenspath or the Anthropic private-attr path, so the regression went undetected.Summary
_cache_bucketsfor cache-read tokens:prompt_tokens_details.cached_tokens->_cache_read_input_tokens(private attr) ->cached_tokens(model_extra)prompt_tokens_details.cache_creation_tokens(guarded bymodel_fields_set) ->_cache_creation_input_tokens(private attr)cached_tokens, Anthropic_cache_read_input_tokens, and Anthropic_cache_creation_input_tokensIssue Number
N/A — found during weekly regression hunt of PR #4968.
How to Test
Before (on unmodified main, commit 76e9e25):
After (with this fix):
Full suite verification:
$ uv run pytest tests/sdk/llm/test_llm_telemetry.py -v ============================== 45 passed in 0.35s ==============================Pre-commit hooks:
$ uv run pre-commit run --files openhands-sdk/openhands/sdk/llm/utils/telemetry.py tests/sdk/llm/test_llm_telemetry.py # All hooks passed (ruff format, ruff lint, pycodestyle, pyright, forbidden dynamic attributes, import rules, tool registration)Video/Screenshots
Not applicable — this is a backend telemetry fix with no UI changes. Test output above serves as reproduction evidence.
Design Doc
Not applicable — narrow fix restoring previously-working behavior.
Type
Notes
model_fields_setforcache_creation_tokens) while restoring the fallback chain that was inadvertently removed.OpenHands/software-agent-sdkrepository.🐳 Agent Server images for this PR — GHCR package, pull/run commands, and all pushed tags (click to expand)
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.13-nodejs22-slimnikolaik/python-nodejs:python3.13-nodejs22-slimgolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:1d220d0-pythonRun
All tags pushed for this build
About Multi-Architecture Support
1d220d0-python) is a multi-arch manifest supporting both amd64 and arm641d220d0-python-amd64) are also available if needed